Set conntrack params in kube-proxy #19182

thockin · 2015-12-29T23:40:48Z

Add flags to control max connections (set to 256k vs 64k default) and TCP
established timeout (set to 1 day vs 5 day default). Flags can be set to 0 to
mean "don't change it".

This is only set at startup, and not wrapped in a rectifier loop.

Tested manually.

Fixes #18604

k8s-bot · 2015-12-29T23:43:15Z

GCE e2e build/test failed for commit 68492f066dfa8f75fa322c0495330fc7bad9a470.

k8s-github-robot · 2015-12-29T23:49:41Z

Labelling this PR as size/L

ArtfulCoder · 2015-12-30T04:00:39Z

cmd/kube-proxy/app/conntrack.go

+	return ioutil.WriteFile("/sys/module/nf_conntrack/parameters/hashsize", []byte(strconv.Itoa(max/4)), 0640)
+}
+
+func (realConntracker) SetTCPEstablishedTimeout(seconds int) error {


should the type be Duration instead of int ?

I chose seconds because any finer granularity is not respected. I could go either way, but given the very limited exposure of this, I think simpler is better.

sounds good

k8s-bot · 2015-12-30T19:59:05Z

GCE e2e test build/test passed for commit 407c744906ee962039f8b9a6285bc20e0d458acb.

k8s-bot · 2015-12-30T19:59:33Z

GCE e2e test build/test passed for commit 2eede77cfeff21e6c21c6ea05cdeffb8dafcf79a.

k8s-bot · 2015-12-30T22:39:25Z

GCE e2e test build/test passed for commit 5263e2638fd29eec00755a42ae88c2abad6a20c6.

Add flags to control max connections (set to 256k vs 64k default) and TCP established timeout (set to 1 day vs 5 day default). Flags can be set to 0 to mean "don't change it". This is only set at startup, and not wrapped in a rectifier loop. Tested manually.

k8s-bot · 2015-12-31T00:16:04Z

GCE e2e test build/test passed for commit da0ac31.

thockin · 2015-12-31T04:33:00Z

@k8s-bot unit test this please

thockin · 2015-12-31T06:25:37Z

@k8s-bot unit test this please

thockin · 2015-12-31T06:49:33Z

@k8s-bot unit test this please

ArtfulCoder · 2015-12-31T21:26:49Z

@k8s-bot unit test this please

a unit test seems to have failed. re-running test to confirm

thockin · 2015-12-31T22:27:08Z

@k8s-bot unit test this please

A different test fails every time. This is intolerable

thockin · 2015-12-31T23:08:09Z

w00t! Green at last. Green at last! Thank git alrighty it is green at last.'

k8s-github-robot · 2015-12-31T23:38:27Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

thockin · 2015-12-31T23:59:52Z

@k8s-bot unit test this please

k8s-bot · 2016-01-01T00:06:26Z

GCE e2e test build/test passed for commit da0ac31.

thockin · 2016-01-01T02:17:08Z

@k8s-bot unit test this please

thockin · 2016-01-01T03:00:48Z

@k8s-bot unit test this please

thockin · 2016-01-01T03:47:40Z

This is hopeless - even if I get it to go green, the chances of it being green again for Jenkins is 0.

@k8s-bot unit test this please

thockin · 2016-01-01T22:18:32Z

@k8s-bot unit test this please

wojtek-t · 2016-01-02T08:30:51Z

@k8s-bot unit test this please

k8s-github-robot · 2016-01-02T08:56:37Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-01-02T09:22:12Z

GCE e2e test build/test passed for commit da0ac31.

wojtek-t · 2016-01-02T09:27:34Z

@k8s-bot unit test this please

k8s-github-robot · 2016-01-02T10:05:38Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-01-02T10:35:56Z

GCE e2e test build/test passed for commit da0ac31.

k8s-github-robot · 2016-01-02T10:36:18Z

Automatic merge from submit-queue

Auto commit by PR queue bot

karlkfi · 2016-01-05T00:44:57Z

Kubernetes-Mesos CI smoke tests have been failing on master since this was merged. Guess that CI failure up there wasn't just a red herring!

Haven't figured out why this broke the tests, but it didn't just break them, it stopped the JUnit test report from being produced too...

karlkfi · 2016-01-05T02:44:26Z

Mesos smoke tests are fixed by #19277. Kinda stupid that it doesn't work without the new flags specified.

ghost · 2016-01-05T17:39:40Z

@thockin @ArtfulCoder sorry to chime in so late on this one. A few words of caution:

From my experience, increasing the size of the conntrack table does not often help matters. An application that initiates an essentially unlimited number connections in parallel (think web crawler) will typically blow this table, whether it's 64K, 256K or even larger. A better approach is to rate limit the outgoing connection establishment packets.
Increasing the conntrack table size requires significant extra kernel memory, so you typically need to tune some other kernel parameters too (I forget which) to make the larger table fit. It might have fitted by pure luck in the particular kernel config that you tested on, but it's quite possible that it will cause other kernels to explode. Distro managers (Redhat, Ubuntu etc) typically tune all the necessary parameters correctly to work in harmony, so messing with individual ones can be dangerous.

jdef · 2016-01-05T21:59:53Z

cmd/kube-proxy/app/conntrack.go

+	}
+	// TODO: generify this and sysctl to a new sysfs.WriteInt()
+	glog.Infof("Setting conntrack hashsize to %d", max/4)
+	return ioutil.WriteFile("/sys/module/nf_conntrack/parameters/hashsize", []byte(strconv.Itoa(max/4)), 0640)


this line appears to be the culprit that fails our smoke testing when max is non-zero.
xref mesosphere/kubernetes-mesos#724

for posterity: the nf_conntrack module doesn't seem to support setting the value of this hashsize parameter for network namespace other than init_net; this is strictly incompatible with our mesos/docker-based testing environment.

Thanks for tracking it - what is the fix? What can we do?

On Wed, Jan 6, 2016 at 8:54 AM, James DeFelice notifications@github.com
wrote:

In cmd/kube-proxy/app/conntrack.go
#19182 (comment)
:

+type Conntracker interface {

SetMax(max int) error

SetTCPEstablishedTimeout(seconds int) error
+}

+type realConntracker struct{}
+
+func (realConntracker) SetMax(max int) error {

glog.Infof("Setting nf_conntrack_max to %d", max)

if err := sysctl.SetSysctl("net/netfilter/nf_conntrack_max", max); err != nil {

return err

}

// TODO: generify this and sysctl to a new sysfs.WriteInt()

glog.Infof("Setting conntrack hashsize to %d", max/4)

return ioutil.WriteFile("/sys/module/nf_conntrack/parameters/hashsize", []byte(strconv.Itoa(max/4)), 0640)

for posterity: the nf_conntrack module doesn't seem to support setting the
value of this hashsize parameter for network namespace other than init_net;
this is strictly incompatible with our mesos/docker-based testing
environment.

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/19182/files#r48979830.

for k8s-mesos I've disabled these tuning parameters by default (read: zero by default). this fixes our CI environment immediately. users can still tweak them if needed/wanted. short of changing the way hashsize is implemented in the kernel module i'm not sure how else to really "fix" this.

More links I collected for the context:

k8s-mesos issue @jdef mentioned, that might be related to setting hashsize inside nested network namespace
kube-proxy connection tracking adjustments are crashing smoke tests mesosphere/kubernetes-mesos#724

more general discussion regarding whether sysctl are namespace safe
Document per namespace sysctl and how to set them in pods #29572

initial Linux kernel change, that restrict conntrack hash resize to init_net
https://lwn.net/Articles/375395/

thockin · 2016-01-20T08:16:31Z

On Tue, Jan 5, 2016 at 9:40 AM, Quinton Hoole notifications@github.com wrote:

@thockin @ArtfulCoder sorry to chime in so late on this one. A few words of caution:

From my experience, increasing the size of the conntrack table does not often help matters. An application that initiates an essentially unlimited number connections in parallel (think web crawler) will typically blow this table, whether it's 64K, 256K or even larger. A better approach is to rate limit the outgoing connection establishment packets.

I don't disagree. UDP is particularly unpleasant because we just have
to leave it to linger.

Increasing the conntrack table size requires significant extra kernel memory, so you typically need to tune some other kernel parameters too (I forget which) to make the larger table fit. It might have fitted by pure luck in the particular kernel config that you tested on, but it's quite possible that it will cause other kernels to explode. Distro managers (Redhat, Ubuntu etc) typically tune all the necessary parameters correctly to work in harmony, so messing with individual ones can be dangerous.

We tune the hash size, too, following best practices, though
apparently this breaks in Mesos.

thockin assigned ArtfulCoder Dec 29, 2015

googlebot added the cla: yes label Dec 29, 2015

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 29, 2015

ArtfulCoder reviewed Dec 30, 2015
View reviewed changes

thockin force-pushed the kube-proxy-conntrack-max branch 2 times, most recently from 2eede77 to 407c744 Compare December 30, 2015 19:21

thockin force-pushed the kube-proxy-conntrack-max branch from 407c744 to 5263e26 Compare December 30, 2015 22:00

thockin force-pushed the kube-proxy-conntrack-max branch from 5263e26 to da0ac31 Compare December 30, 2015 23:46

ArtfulCoder added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 31, 2015

k8s-github-robot pushed a commit that referenced this pull request Jan 2, 2016

Merge pull request #19182 from thockin/kube-proxy-conntrack-max

4f65f68

Auto commit by PR queue bot

k8s-github-robot merged commit 4f65f68 into kubernetes:master Jan 2, 2016

thockin deleted the kube-proxy-conntrack-max branch January 4, 2016 04:33

jdef mentioned this pull request Jan 5, 2016

MESOS: disable conntrack behavior mods that are possibly causing smoke tests to bomb #19277

Merged

jdef reviewed Jan 5, 2016
View reviewed changes

jdef mentioned this pull request Jan 5, 2016

MESOS: avoid updating nf_conntrack_max and _hashsize settings by default #19303

Merged

Random-Liu mentioned this pull request Jul 8, 2016

Work around KubeProxy panic that makes Node's network not working #25543

Closed

Set conntrack params in kube-proxy #19182

Set conntrack params in kube-proxy #19182

Conversation

thockin commented Dec 29, 2015

k8s-bot commented Dec 29, 2015

k8s-github-robot commented Dec 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Dec 30, 2015

k8s-bot commented Dec 30, 2015

k8s-bot commented Dec 30, 2015

k8s-bot commented Dec 31, 2015

thockin commented Dec 31, 2015

thockin commented Dec 31, 2015

thockin commented Dec 31, 2015

ArtfulCoder commented Dec 31, 2015

thockin commented Dec 31, 2015

thockin commented Dec 31, 2015

k8s-github-robot commented Dec 31, 2015

thockin commented Dec 31, 2015

k8s-bot commented Jan 1, 2016

thockin commented Jan 1, 2016

thockin commented Jan 1, 2016

thockin commented Jan 1, 2016

thockin commented Jan 1, 2016

wojtek-t commented Jan 2, 2016

k8s-github-robot commented Jan 2, 2016

k8s-bot commented Jan 2, 2016

wojtek-t commented Jan 2, 2016

k8s-github-robot commented Jan 2, 2016

k8s-bot commented Jan 2, 2016

k8s-github-robot commented Jan 2, 2016

karlkfi commented Jan 5, 2016

karlkfi commented Jan 5, 2016

ghost commented Jan 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Jan 20, 2016